Feature proposal: Editable Diff #1882
Conversation
Co-Authored-By: simurai <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1882 +/- ##
==========================================
+ Coverage 92.91% 92.95% +0.04%
==========================================
Files 219 219
Lines 12570 12570
Branches 1798 1798
==========================================
+ Hits 11679 11685 +6
+ Misses 891 885 -6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
thanks for writing this up, @vanessayuenn ! It's thorough and you covered a lot of edge cases. Really excited about this feature.
| - use grey/void blocks to reconcile the line differences between the two sides so they line up properly | ||
|
|
||
| ##### Pros: | ||
| - it's easy to understand that one side is editable, and the other is not. |
|
|
||
| - Currently, if a user makes changed to a staged file, the new changes show up in Unstaged Changes, but are not applied to the already staged file. If we allow a staged file to be edited, should the new changes apply to both file on disk as well as the staged entry? | ||
|
|
||
| - The PR comments we are implementing in [#1856](https://github.com/atom/github/pull/1856) already add considerable complexity to the diff view. Should the comments be visible when the diff is in an editable state? |
There was a problem hiding this comment.
I believe comments should be visible when the diff is in an editable state, because it would be really useful to our users to be able to address comments while also seeing the comments inline.
There was a problem hiding this comment.
I agree. 👍 Reading a comment and then be able to make changes right there without switching to some sort of "edit mode" would be nice.
|
|
||
| ## :question: Unresolved questions | ||
|
|
||
| - Currently, if a user makes changed to a staged file, the new changes show up in Unstaged Changes, but are not applied to the already staged file. If we allow a staged file to be edited, should the new changes apply to both file on disk as well as the staged entry? |
There was a problem hiding this comment.
Yes! Be able to edit staged changes would make the "All staged changes" diff (commit preview) more useful. Like before committing, you might spot a typo and don't have to 1. unstage 2. edit 3. stage again and can directly make the correction. Although it's probably a bit against our "stay true to Git" vision. 😬
|
|
||
| - Currently, if a user makes changed to a staged file, the new changes show up in Unstaged Changes, but are not applied to the already staged file. If we allow a staged file to be edited, should the new changes apply to both file on disk as well as the staged entry? | ||
|
|
||
| - The PR comments we are implementing in [#1856](https://github.com/atom/github/pull/1856) already add considerable complexity to the diff view. Should the comments be visible when the diff is in an editable state? |
There was a problem hiding this comment.
I agree. 👍 Reading a comment and then be able to make changes right there without switching to some sort of "edit mode" would be nice.
|
|
||
| - The PR comments we are implementing in [#1856](https://github.com/atom/github/pull/1856) already add considerable complexity to the diff view. Should the comments be visible when the diff is in an editable state? | ||
|
|
||
| - When modifying a diff, should actions such as stage/unstage, jump to file, etc. trigger a "save"? |
There was a problem hiding this comment.
should actions such as stage/unstage
I'm a bit on the fence, but tend towards a "no". It might depends on the "can staged changes be edited?" question above.
| - The cursor for hovering over deleted lines will show up as an arrow as opposed to a text cursor. | ||
| - Deleted lines can still be selected (via mouse or keyboard) but no caret will be visible; basically same as current behaviour. | ||
|  | ||
| - As a user makes edits in a diff view, the file name will have some visual cue indicating there are unsaved modifications, akin to when a regular atom editor has unsaved changes. |
There was a problem hiding this comment.
Might be useful to also show the modified indicator (blue dot) on the tab? Just like for regular files. Then it's easier to see if a diff has unsaved edits when it's not the active tab or the file header is "scrolled out of view".
I might be even enough to only have the indicator on the tabs, even for multi-file diffs.
vanessayuenn
changed the title
This is a work-in-progress feature proposal, started off as a summary of the discussion in #1720. As I did more research and thought more about the execution, however, I realized making a unified diff view editable is quite a bit more challenging than originally anticipated. That probably is why there's no prior art of similar implementation to be found in the wild (all of the editable diff tool uses the split view approach which is covered in this proposal 😄)!
A chat with @simurai this morning has inspired a slightly different direction, which is what this RFC is proposing. I have included the first iteration as well in "Rationale and alternatives" section for the record. The last few sections are TBD at the moment but will be filled out as discussions unfold.
View rendered docs/feature-requests/005-editable-diff.md